Skip to content

Upgrade Substrate and Frontier#1954

Merged
nazar-pc merged 13 commits intomainfrom
upgrade-substrate
Sep 13, 2023
Merged

Upgrade Substrate and Frontier#1954
nazar-pc merged 13 commits intomainfrom
upgrade-substrate

Conversation

@nazar-pc
Copy link
Copy Markdown
Member

@nazar-pc nazar-pc commented Sep 13, 2023

Frontier updates are done in polkadot-evm/frontier#1193, Substrate updates are done in subspace-v6 branch in our fork (rebased due to various conflicts, primarily with block relay).

This upgrades us to latest commit in the old Substrate repo, I'll upgrade us to latest polkadot-sdk in a separate PR.

Interesting changes:

Other important upstream changes that resulted in downstream modifications:

paritytech/substrate#14709 was promising, but not sufficient, so I opened paritytech/polkadot-sdk#1533

Code contributor checklist:

vedhavyas
vedhavyas previously approved these changes Sep 13, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@vedhavyas vedhavyas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

construct_runtime seems much more readable now.

LGTM

@nazar-pc
Copy link
Copy Markdown
Member Author

nazar-pc commented Sep 13, 2023

construct_runtime seems much more readable now.

Recall what it looked like 2 years ago. I hope they will get to attribute macro there at some point too 🙂

@nazar-pc nazar-pc enabled auto-merge September 13, 2023 09:35
@nazar-pc nazar-pc disabled auto-merge September 13, 2023 09:51
@nazar-pc
Copy link
Copy Markdown
Member Author

nazar-pc commented Sep 13, 2023

Okay, looks like something else related to transaction pool needs to be changed to fix tests 😕

@nazar-pc
Copy link
Copy Markdown
Member Author

nazar-pc commented Sep 13, 2023

@vedhavyas @NingLin-P I'll need your help fixing domains tests here

@nazar-pc
Copy link
Copy Markdown
Member Author

nazar-pc commented Sep 13, 2023

Interesting, looks like to be able to submit transactions this is now required:

let mut runtime_api = self.client.runtime_api();
runtime_api.register_extension(
    offchain_tx_pool_factory.offchain_transaction_pool(best_hash),
);

Let me fix this and see if tests pass.

NingLin-P
NingLin-P reviewed Sep 13, 2023
View reviewed changes
vedhavyas
vedhavyas previously approved these changes Sep 13, 2023
View reviewed changes
@NingLin-P
Copy link
Copy Markdown
Contributor

NingLin-P commented Sep 13, 2023

Is it possible to register the extension just once like main branch does instead of registering the extension whenever we need to submit tx?

@nazar-pc
Copy link
Copy Markdown
Member Author

nazar-pc commented Sep 13, 2023

I believe it should be possible, but it is also recommended to not do that.

rahulksnv
rahulksnv previously approved these changes Sep 13, 2023
View reviewed changes
@vedhavyas
upgrade frontier to latest commit.
a7c09c6 
We had to introduce Inherent data providers since upstream introduced `pending` tags for Eth rpc calls.
When an rpc call with pending tag is received, we construct pending block by building a new block with extrinsics from tx pool.
So they use inherent data providers to provide inherent data. For now, we just provide timestamp since that is always required.
I have added an TODO to handle any new inherents that needs to be present. Ex runtime upgrade can be added in later PR
@nazar-pc nazar-pc dismissed stale reviews from rahulksnv and vedhavyas via 7a478d9 September 13, 2023 19:22
vedhavyas
vedhavyas previously approved these changes Sep 13, 2023
View reviewed changes
@nazar-pc
Copy link
Copy Markdown
Member Author

nazar-pc commented Sep 13, 2023

Hm, merge conflicts already 😒

@nazar-pc nazar-pc mentioned this pull request Sep 13, 2023
Closed
1 task
@nazar-pc nazar-pc enabled auto-merge September 13, 2023 19:36
@nazar-pc nazar-pc merged commit 498c4a2 into main Sep 13, 2023
@nazar-pc nazar-pc deleted the upgrade-substrate branch September 13, 2023 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rg3l3dr rg3l3dr Awaiting requested review from rg3l3dr

@shamil-gadelshin shamil-gadelshin Awaiting requested review from shamil-gadelshin

@ParthDesai ParthDesai Awaiting requested review from ParthDesai

@vedhavyas vedhavyas Awaiting requested review from vedhavyas vedhavyas is a code owner

2 more reviewers

@NingLin-P NingLin-P NingLin-P approved these changes

@rahulksnv rahulksnv rahulksnv left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@nazar-pc @NingLin-P @vedhavyas @rahulksnv